[18RoyalGorge] Require graph connection for cross-buying trains #11386
Conversation
From page 13 of 24 in the [current rulebook PDF][0]. Check for the connection when processing the action--instead of when finding available trains to buy--to minimize the necessary graph checks. Fixes tobymao#11364 [0]: https://boardgamegeek.com/filepage/284918/graphic-rulebook-preliminary
| @@ -16,6 +16,21 @@ def room?(entity) | |||
| # even when train tight, there's room for a self-rust | |||
| return true if entity.trains.any? { |t| t.rusts_on == upcoming_train.name } | |||
| end | |||
|
|
|||
| def check_connected!(buyer, train) | |||
| return if train.owner == @game.depot | |||
There was a problem hiding this comment.
Adding a return if @game.loading guard cause here would remove the need for the game graph to be recalculated when a game is being loaded, at the cost of not rechecking the legality of the move.
There was a problem hiding this comment.
I agree that the check should be omitted if loading.
|
|
||
| seller = train.corporation | ||
| seller_nodes = @game.graph.connected_nodes(seller) | ||
| return if @game.graph.connected_nodes(buyer).any? { |node, _| seller_nodes.include?(node) } |
There was a problem hiding this comment.
Would it be more efficient to do return unless (@game.graph.connected_nodes(train.corporation) & @game.graph.connected_nodes(buyer)).empty?
There was a problem hiding this comment.
graph.connected_nodes is a hash, and so doesn't have a & method. You could do it as:
@game.graph.connected_nodes(train.corporation).keys.intersect?(@game.graph.connected_nodes(buyer).keys)
But I'm not sure if that's clearer.
|
@michaeljb This pull request has been open for three months now. Have you got any updates for this? |
From page 13 of 24 in the current rulebook PDF.
Check for the connection when processing the action--instead of when finding available trains to buy--to minimize the necessary graph checks.
Fixes #11364
Pins needed: any games where an illegal (unconnected) cross-buy occurred.
Before clicking "Create"
masterpinsorarchive_alpha_gameslabel if this change will break existing gamesdocker compose exec rack rubocop -adocker compose exec rack rake